Skip to content

Conversation

@jacomago
Copy link
Contributor

@jacomago jacomago commented Jan 10, 2025

Adds the ruff linter and formatter https://docs.astral.sh/ruff/

Checks for problematic code and auto formats code.

Removes unused code

@jacomago jacomago force-pushed the ruff branch 2 times, most recently from 5562d04 to 47ca0f1 Compare January 10, 2025 14:28
@jacomago jacomago self-assigned this Jan 23, 2025
@jacomago jacomago marked this pull request as ready for review January 23, 2025 11:44
pvInfo[rid] = {"pvName": rname}
if (self.conf.get('recordType', 'default' == 'on')):
pvInfo[rid]['recordType'] = rtype
if self.conf.get("recordType", "default" == "on"):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks... weird. Should this be instead

if self.conf.get("recordType", default="on"):

Otherwise, this is just equivalent to

if self.conf.get("recordType", False):

???

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by swapping to just if self.conf.get("recordType")


# 0x0002
def recvPong(self, body):
nonce, = _ping.unpack(body[:_ping.size])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks odd to me, why is it not just

nonce = _ping.unpack(...)

Or is the point that it is a list with a single element? I guess then

nonce, *_ = _ping.unpack(...)

but maybe this doesn't matter...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think unpack returns a tuple (can actually see in the docs)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I though the convention was to prefer foo, over (foo,)

Pretty clear this has been broken for the "alias" and "recordType" settings for a long time.
I've set both options now that if they are set, then they are on. I expect most people turn both options on.
@sonarqubecloud
Copy link

@jacomago jacomago requested a review from simon-ess March 3, 2025 14:21
Copy link
Contributor

@anderslindho anderslindho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


# 0x0002
def recvPong(self, body):
nonce, = _ping.unpack(body[:_ping.size])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I though the convention was to prefer foo, over (foo,)

@jacomago jacomago merged commit c9eaf55 into ChannelFinder:master Mar 7, 2025
62 checks passed
@jacomago jacomago deleted the ruff branch March 10, 2025 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants